This seems to be ignoring the entire point of adding NotifyTask.message_group_id in alphagov/notifications-utils#1318

The point is that you don't have to go digging around everywhere to find the id you want to use for MessageGroupId, you just feed it in to the first celery task you send to the queue and in other celery tasks you simply forward from self.message_group_id to MessageGroupId=....

I did all of this in https://github.com/alphagov/notifications-api/pull/4693/changes#diff-cca5917c620894fe770faa88835e9e186285d5b91a55fca992eb94d5dc87e635

This seems to be ignoring the entire point of adding NotifyTask.message_group_id in alphagov/notifications-utils#1318

The point is that you don't have to go digging around everywhere to find the id you want to use for MessageGroupId, you just feed it in to the first celery task you send to the queue and in other celery tasks you simply forward from self.message_group_id to MessageGroupId=....

I did all of this in https://github.com/alphagov/notifications-api/pull/4693/changes#diff-cca5917c620894fe770faa88835e9e186285d5b91a55fca992eb94d5dc87e635

@risicle the point of entry for a task needs an explicit message_group_id (in this case service_id) right?

if you're re-enqueing the task again using "self" is valid (like I did for retries)

at some point in the journey, we need an explicit grouping, otherwise where does it get it from? we need to pass on service_id

See my PR, it gets done in run_scheduled_jobs and create_job (in app/job/rest.py), because process_job itself also has a service_id context, which the shattered tasks can inherit their group id from.

We need to feed the message group id in at the point where a process becomes service_id-specific, ideally nowhere else.

We need to feed the message group id in at the point where a process becomes service_id-specific, ideally nowhere else.

@risicle done

I think we need to inject the MessageGroupId in run_scheduled_jobs and check_for_missing_rows_in_completed_jobs too - the two other places job-related tasks get initiated (see https://github.com/alphagov/notifications-api/pull/4693/changes#diff-859b3fa7698d3dc1be32ef99742c5d8620fab88d231fb3e4078752f7dc5bc05f)

FWIW this is why I'm unsure about whether we should be adding MessageGroupId bit-by-bit rather than just doing everything all at once like I did in my demo PR. We're liable to miss bits this way, and it's not like we'll be able to use ENABLE_SQS_MESSAGE_GROUP_IDS to enable the different parts separately - it's a single global flag.

I think this is right, but if we're going to make it default to ENABLE_SQS_MESSAGE_GROUP_IDS = 1 in the code we should update notifications-aws to set it to 0 in the terraform before we merge this.

@risicle https://github.com/alphagov/notifications-aws/pull/2880